Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updated the fail() exceptions with Junit5. #159

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Updated the fail() exceptions with Junit5. #159

wants to merge 21 commits into from

Conversation

dota17
Copy link
Contributor

@dota17 dota17 commented May 7, 2020

No description provided.

@coveralls
Copy link

coveralls commented May 8, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.004%) to 90.123% when pulling 46ad29e on dota17:junit5WithExceptions into cec45f8 on apache:master.

@garydgregory garydgregory changed the title [WIP]Updated the fail() exceptions with Junit5. [WIP] Updated the fail() exceptions with Junit5. May 12, 2020
@garydgregory
Copy link
Member

@dota17 The title says "WIP". Are you still working on it?

@dota17 dota17 changed the title [WIP] Updated the fail() exceptions with Junit5. Updated the fail() exceptions with Junit5. May 19, 2020
@dota17
Copy link
Contributor Author

dota17 commented May 21, 2020

@dota17 The title says "WIP". Are you still working on it?

Yes, I am.
If anyone has time to review, it would be really nice. ;-)

Thanks

@garydgregory
Copy link
Member

@dota17 The title says "WIP". Are you still working on it?

Yes, I am.
If anyone has time to review, it would be really nice. ;-)

Thanks

Hi @dota17
This PR currently touches 124 files, which is a lot to review. Personally, for me to review this, I'd want to know the PR is as best as you can make it, because, as you can imagine, I really do not want to do it twice! ;-) So either it's WIP or it is as good as it is going to get from your POV at least. Please let us know...

@dota17
Copy link
Contributor Author

dota17 commented May 23, 2020

Hi @garydgregory
Under the folder
commons-collections/src/test/java/org/apache/commons/collections4 (junit5WithExceptions),
I executed the command to get the fails without annotate and import.
grep -R fail * | grep -v import |grep -v "\/\/" | grep -v "*"
The last 58 lines only use the fail without exception to get the failure details.
I had used JUnit 5's assertThrows() in the tests instead of fail() or Assert.fail() in the
132 files.

@dota17
org.junit.jupiter.api.Assertions#assertThrows was used to assert that…
7b243cb 
… a specific exception was thrown instead of an org.junit.Test annotation with an expected
@dota17
Remove the unused import.
ccb7eab
@dota17
Copy link
Contributor Author

dota17 commented May 28, 2020

Hi @garydgregory
I want to migrate to JUnit5 Jupiter. Such as the pr :
apache/commons-csv#49
Is it OK ?

@dota17
Annotations:
d088d2d 
org.junit.jupiter.api.Test was used as a drop in replacement for org.juit.Test without arguments. See 3.ii. for handling of @test annotations with an expected argument.
org.junit.jupiter.api.BeforeEach was used as a drop in replacement for org.junit.Before.
org.junit.jupiter.api.BeforeAll was used as a drop in replacement for org.junit.BeforeClass.
org.junit.jupiter.api.Disabled was used as a drop in replacement for org.junit.Ignore.
@dota17
Merge branch 'master' into junit5WithExceptions
d74240b
@dota17
Copy link
Contributor Author

dota17 commented May 30, 2020

comparators/AbstractComparatorTest.java
comparators/BooleanComparatorTest.java
comparators/ComparatorChainTest.java
comparators/FixedOrderComparatorTest.java
comparators/ReverseComparatorTest.java
iterators/BoundedIteratorTest.java
iterators/PeekingIteratorTest.java
iterators/PushbackIteratorTest.java
iterators/SkippingIteratorTest.java
keyvalue/AbstractMapEntryTest.java
keyvalue/DefaultKeyValueTest.java
keyvalue/DefaultMapEntryTest.java
keyvalue/TiedMapEntryTest.java
keyvalue/UnmodifiableMapEntryTest.java
map/LazyMapTest.java
map/LazySortedMapTest.java
properties/AbstractPropertiesFactoryTest.java
properties/PropertiesFactoryTest.java
properties/SortedPropertiesFactoryTest.java

org.junit.jupiter.api.Test was used as a drop in replacement for org.juit.Test without arguments. These Tests are with arguments, how to proceed?

kinow
kinow reviewed Aug 26, 2020
View reviewed changes
Copy link
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dota17 conflicts in this PR 👍

@garydgregory
Copy link
Member

I just merged a series of JUnit 5 PRs; if this PR still contains changes not covered in git master, please rebase, if not, please close it. TY!

@garydgregory
Copy link
Member

I think JUnit 5 work might be done in master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kinow kinow kinow left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dota17 @coveralls @garydgregory @kinow